## Optimize validateParentPath for Faster Parent Path Validation
#1396
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit refactors the `validateParentPath` function in `transaction-shim.ts` to improve its performance, especially for deep or frequent path validations during storage transactions. ### Optimization Details - **Single-pass Traversal:** The previous implementation redundantly checked object-ness and existence at each path level, sometimes re-traversing the same objects. The new approach streamlines the traversal by iterating through the parent path only once, directly walking down the value tree and stopping at the first missing or non-object parent. - **Efficient Error Handling:** Error construction is now more precise, and the function avoids unnecessary object checks and array slicing. The logic for setting the error's `.path` property is simplified, reducing overhead in error cases. - **No Change in Behavior:** All existing functionality and error semantics are preserved. The function still returns the same errors for invalid paths, but does so with less computational overhead.
There was a problem hiding this comment.
cubic found 2 issues across 1 file. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| nextValue = nextValue[key] = typeof Number(key) === "number" ? [] : {}; | ||
| nextValue = | ||
| nextValue[key] = | ||
| (typeof Number(key) === "number" ? [] : {}) as typeof nextValue; |
There was a problem hiding this comment.
typeof Number(key) === "number" is always true, so the code always creates an array and never an object, corrupting the structure when key is non-numeric
Suggested change
| (typeof Number(key) === "number" ? [] : {}) as typeof nextValue; | |
| (Number.isInteger(Number(key)) ? [] : {}) as typeof nextValue; |
| pathError.path.push(segment); | ||
| } | ||
| } | ||
| if (parentIndex > 0) pathError.path = path.slice(0, parentIndex - 1); |
There was a problem hiding this comment.
Off-by-one in slice: omits the last valid segment, so pathError.path reports an empty or incomplete parent path
Suggested change
| if (parentIndex > 0) pathError.path = path.slice(0, parentIndex - 1); | |
| if (parentIndex > 0) pathError.path = path.slice(0, parentIndex); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit refactors the
validateParentPathfunction intransaction-shim.tsto improve its performance, especially for deep or frequent path validations during storage transactions.Optimization Details
Single-pass Traversal: The previous implementation redundantly checked object-ness and existence at each path level, sometimes re-traversing the same objects. The new approach streamlines the traversal by iterating through the parent path only once, directly walking down the value tree and stopping at the first missing or non-object parent.
Efficient Error Handling: Error construction is now more precise, and the function avoids unnecessary object checks and array slicing. The logic for setting the error's
.pathproperty is simplified, reducing overhead in error cases.No Change in Behavior: All existing functionality and error semantics are preserved. The function still returns the same errors for invalid paths, but does so with less computational overhead.
Summary by cubic
Refactored the validateParentPath function to validate parent paths in a single pass, improving performance for deep or frequent storage transactions.